Fix CI workloads and enhance error handling #782
Conversation
aklenik
requested review from
feihujiang,
jiangyaoguo,
liam-grace,
nklincoln and
panyu4
as code owners
| @@ -365,7 +365,7 @@ class Sawtooth extends BlockchainInterface { | |||
| * @return {Promise} The return promise. | |||
| */ | |||
| getContext(name, args) { | |||
| let config = require(this.configPath); | |||
| let config = require(configPath); | |||
There was a problem hiding this comment.
This one kinda makes me itchy. I think the better solution here would be to set the config inside the Sawtooth constructor, and move the functions into the class so that they have access to a this.config property. Moreover, this would remove dynamic requires in the "hot path" during invokeSmartContract
nklincoln
added
the
PR/change requested
Signed-off-by: Attila Klenik <[email protected]>
|
@nklincoln I decided to make the config object a global variable, just like the configPath was. It was easier than refactoring the entire adapter 😅 |
@aklenik It looks like these changes incorporate my work from #780 correctly. Does making the |
@benjamincburns You are right, the structure of the module could be improved (by making it more OO). It should be a trivial refactoring, but currently, most of our efforts go into cleaning up and enhancing the core code, and not the adapters (yet). But feel free to refactor it if you have some extra free time :) Although, working on the Ethereum/Besu adapter is probably more useful based on the community activity. |
|
ok, I'll raise a separate issue to refactor the sawtooth adaptor 👍 |
|
Also, while in sawtooth code you missed a couple of |
Signed-off-by: Attila Klenik [email protected]
bcType=>getType()fix is applied to the Ethereum and Besu CI workload modules.